-
Notifications
You must be signed in to change notification settings - Fork 71
Refonte Trésorie > Devis - Ajout / Edition #2016
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
resolves #2002 |
686683e to
15cc2a5
Compare
556dddb to
0034d1f
Compare
sources/AppBundle/Accounting/Model/Repository/InvoicingRepository.php
Outdated
Show resolved
Hide resolved
sources/AppBundle/Controller/Admin/Accounting/Quotation/EditQuotationAction.php
Outdated
Show resolved
Hide resolved
| return $this; | ||
| } | ||
|
|
||
| public function isValid(): bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi ne pas utiliser une contrainte de validation ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm et en plus ça veut dire que la classe permet un état invalide, les colonnes en base ne sont pas nullables.
0034d1f to
eef10b5
Compare
eef10b5 to
257149a
Compare
stakovicz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gros boulot ! Bravo !
Il reste aussi des commentaires non résolus.
sources/AppBundle/Controller/Admin/Accounting/Quotation/EditQuotationAction.php
Outdated
Show resolved
Hide resolved
1ff4e5c to
9f239ee
Compare
9f239ee to
7e2ca90
Compare
|
@stakovicz @Mopolo j'ai implémenté une solution plus propre que le "wait Xs" dans les tests behat. Je retente l'assertion toutes les 100ms pendant un temps donné (cf. Afup\Tests\Behat\Bootstrap\WaitContext). |
Mopolo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est beaucoup mieux le wait que t'as fait là !
Faudra voir à l'usage si on besoin de setup du retry auto de ces tests.
Quelques remarques encore mais je trouve qu'on approche du merge :)
| return $this; | ||
| } | ||
|
|
||
| public function isValid(): bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm et en plus ça veut dire que la classe permet un état invalide, les colonnes en base ne sont pas nullables.
| 'scale' => 2, | ||
| ])->add('unitPrice', NumberType::class, [ | ||
| 'label' => 'Prix unitaire HT', | ||
| 'required' => false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vu que les lignes peuvent maintenant être ajoutées dynamiquement, pourquoi ne pas forcer les colonnes non-nullables ?
Là ça veut dire que si je clic sur le bouton pour en ajouter 3 et que j'en rempli 2, la 3ème sera ignorée.
Avec l'ancien système ça ne posait pas de soucis vu que les 5 étaient en dur. Mais là je trouve ça dommage et contre-intuitif, on n'est plus sûr de ce qui sera enregistré ou pas.
| $idsToRemove = $this->invoicingDetailRepository->getRowsIdsPerInvoicingId($quotation->getId()); | ||
| $existingIds = []; | ||
| $this->invoicingRepository->startTransaction(); | ||
| $this->unitOfWork->pushSave($quotation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je ne maitrise pas du tout Ting, c'est nécessaire ça vu qu'il y a ça plus bas ?
$this->invoicingRepository->save($quotation);
J'ai un peu modifié la gestions des lignes de devis sur la page d'ajout/modification d'un devis. On peut maintenant rajouter une ligne au besoin.
Avant :

Après :

La gestion des lignes est faite en javascript, pour pouvoir tester cel, j'ai dû rajouter la possibilité de faire des tests avec un vrai navigatuer. Les tests necessitant du javascript sont pilotés avec Behat via Panther sur le navigateur Chromium.